Skip to content

Add code and test assurance testing #95

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Nov 21, 2017

Conversation

bpostlethwaite
Copy link
Member

@alexcjohnson okay final piece before the merge to master. This PR adds a Jasmine test skip blacklist and both syntactic and logical code checks in CI using prettier and eslint.

@bpostlethwaite bpostlethwaite force-pushed the AddCodeAndTestAssuranceTesting branch 2 times, most recently from 83e7b84 to 0390263 Compare November 21, 2017 17:31
import glob from 'glob';

const BLACK_LIST = ['fdescribe', 'fit', 'xdescribe', 'xit'];
const REGEXS = BLACK_LIST.map(token => new RegExp(`.*${token}\\(.*`));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might need to be a bit stricter, in case someone uses the word "exit", or "fitting" or something... plotly.js uses falafel to make sure we're just matching identifiers:
https://github.com/plotly/plotly.js/blob/master/tasks/test_syntax.js#L28

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeh I tried that but had issues running it inside Jest. I think anchoring the token to the beginning of the line preceded by 0-n spaces should suffice. That works for streambed and this codebase is much smaller.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see... doesn't like jsx perhaps... so maybe .*\W${token}\W.*?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

incidentally if it is jsx that's the problem, there is a solution for it in falafel https://github.com/substack/node-falafel/blob/master/readme.markdown#custom-parser

@bpostlethwaite bpostlethwaite force-pushed the AddCodeAndTestAssuranceTesting branch from 0390263 to 88c08c6 Compare November 21, 2017 17:43
@alexcjohnson
Copy link
Collaborator

💃

@bpostlethwaite bpostlethwaite merged commit 26c2dc8 into develop Nov 21, 2017
@bpostlethwaite bpostlethwaite deleted the AddCodeAndTestAssuranceTesting branch November 21, 2017 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants